Skip to content

Conversation

@justinrosner
Copy link
Contributor

@justinrosner justinrosner commented Oct 17, 2025

Motivation

This PR pulls down a change from upstream LLVM that fixes a crash in SROA that was seen when running the MIGraphX CI: https://github.com/ROCm/rocMLIR-internal/issues/2064

Upstream PR: llvm/llvm-project#162921

Technical Details

The change fixes a bug in the SROA where tree-structured merge optimization was incorrectly applied when the size of the stored value was not a multiple of the new allocated element type size

Test Plan

  • PR CI

Test Result

  • PR CI

Submission Checklist

Copy link
Contributor

@dhernandez0 dhernandez0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, can you add the migraphx failure as a test on our CI as well?

@justinrosner
Copy link
Contributor Author

Note for reviewers: once this gets all reviews and passes the CIs I will separate this into two separate commits (one internal, one external) and then merge without squashing

Comment on lines +4 to +5
if not config.arch.startswith("gfx12"):
config.excludes = ['mixr-tadd-tadd-quant-dot.mlir']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, i've seen this failing on gfx950 as well. Can you try perhaps enabling this test in all the arches ?

Copy link
Contributor Author

@justinrosner justinrosner Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that already. The perfConfig that this test requires to hit the failing case doesn't seem to be valid on all architectures (e.g., gfx942) and won't compile

@justinrosner justinrosner merged commit aeb0021 into develop Oct 18, 2025
9 of 16 checks passed
@justinrosner justinrosner deleted the justinr-sroa-fix branch October 18, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants